Repository navigation
release: v0.1.1 - #7
Merged
Merged
Conversation
…race Replace goroutine+done channel pattern in Subscribe with context.AfterFunc + atomic.Bool.CompareAndSwap. Fixes two bugs (issue #6, reported by @AnyCPU): 1. Goroutine leak: each Subscribe spawned a goroutine that blocked on select{ctx.Done, done} for the subscription lifetime. SubscribeForever (context.Background) parked it forever. 10k subscriptions = 10k idle goroutines (20-40MB stack waste). 2. Race condition: concurrent cancel() + Unsubscribe() could both reach close(done) via non-atomic select-default guard, causing panic: close of closed channel. context.AfterFunc registers cleanup without a goroutine; for context.Background (Done()==nil) nothing is registered at all. atomic.Bool.CompareAndSwap makes cleanup idempotent without the extra heap allocation sync.Once requires. Signal_Subscribe: 745ns/224B/3allocs -> 192ns/52B/2allocs (3.9x) Effect_Stop benchmark: OOM -> 552ns (goroutine accumulation gone) Goroutines per 10k held subs: 10000 -> 0 Closes #6
Four independent optimizations: - Pre-allocate unsubscribes slice with known dep count - Cache AsReadonly() in signal struct (lazy init, same pointer on every call) - Capture e.run method value once before dep loop (avoids N closures for N deps) - Dual-path Effect: store fnSimple directly, skip wrapper closure allocation Effect_Create(1dep): 1560ns/384B/10allocs -> 352ns/196B/7allocs (4.4x) Effect_Create(3deps): 4620ns/1016B/24allocs -> 726ns/388B/13allocs (6.4x) Signal_Subscribe: 745ns/224B/3allocs -> 134ns/52B/2allocs (5.6x) Remaining 7 allocs are near-irreducible: effect struct, subscribe closure+atomic, type-erased callback, method value, slice header.
10 deterministic allocation-count tests that break the build if heap allocations increase: - Signal: Get(0), Set(0), Set+subscriber(1), SubscribeForever(2), Subscribe+ctx(5) - Computed: Get cached(0), SubscribeForever(2) - AsReadonly cached(0) - Effect(7), EffectWithCleanup(7) Unlike benchmark ns/op, AllocsPerRun counts are deterministic and will catch any allocation regression immediately in CI.
- README: update test count (69), coverage (93.8%), benchmarks with post-optimization numbers, remove make/scripts references - CHANGELOG: add v0.1.1 entry (goroutine leak fix, race fix, 5.6x Subscribe speedup, allocation optimizations) - Remove scripts/ directory (unused) - Add tmp/ to .gitignore - Remove /scripts/ from CODEOWNERS
docs/dev/ is in .gitignore — links to it from README and CONTRIBUTING were dead for anyone cloning the repo. Replace with pkg.go.dev reference and inline design notes.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v0.1.1 — Subscribe lifecycle fix + allocation optimization
Fixed
Goroutine leak in Subscribe — each
Subscribecall spawned a goroutine thatblocked for the subscription lifetime.
SubscribeForever(context.Background)parked it forever. 10k subscriptions = 10k idle goroutines (20-40MB stack waste).
Replaced with
context.AfterFunc— zero goroutines per subscription.(signals: drop per-Subscribe goroutine (context.AfterFunc); Once-guard Unsubscribe #6, reported by @AnyCPU)
Race condition in Unsubscribe — concurrent
cancel()+Unsubscribe()couldboth reach
close(done)via non-atomic select-default guard, causingpanic: close of closed channel. Replaced withatomic.Bool.CompareAndSwapfor idempotent cleanup. (signals: drop per-Subscribe goroutine (context.AfterFunc); Once-guard Unsubscribe #6)
Performance
Added
testing.AllocsPerRun— deterministic, breaksbuild if heap allocations increase
and concurrent cancel+unsub race condition
AsReadonly()— returns same pointer on every callEffect— avoids wrapper closure allocation for effects without cleanupRemoved
scripts/directory (unused pre-release check scripts)Test plan
-racego vetclean,gofmtcleanCloses #6